fix(recall): answer --help before working and validate path flags (L3, #139, #140) - #157
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Target branch corrected This pull request now targets The |
github-actions
Bot
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9128bd3cd4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return kind === "chat" ? runChatSearch(argv.slice(2)) : runMemorySearch(argv.slice(2)); | ||
| } | ||
| if (kind === "chat" && sub === "index") { | ||
| if (argv.slice(2).some((a) => a === "help" || a === "/?")) { |
There was a problem hiding this comment.
When a valid path option has the literal relative value help or /?—for example, cxc chat index --home help or --index-path help—this scan mistakes that value for a positional help command, prints usage, and exits successfully without performing the requested index operation. Parse the arguments before recognizing these aliases, or otherwise ensure they are actual positionals rather than string option values.
Useful? React with 👍 / 👎.
L2 moved memory-search.ts by 46 lines, so every line number this PRD quotes for that file is wrong. The dangerous one is 672: it used to be the searchStage1 db warning and is now fillStage1Presence's parameter list, so a blind range apply corrupts a different function. Amendment A re-measures all of them, pins the help check at the top of main because chat index routes to ingest before any sub-command could answer, records that the duplicate warning comes from the two searchStage1 calls rather than two listMarkdownFiles calls, and rejects the cited prior art because it would make memory search help print usage instead of searching for the word.
The reviewer traced sixteen argv shapes through main and found the help check catches every route that reaches ingest today, and no in-repo caller that stricter parsing would break. It also found the planned dash-leading tests would stay red after a correct fix, because Node 24 strict parseArgs throws an ambiguous-argument error before the plan's own message can be produced; those tests are split into the issue repro and the equals-form guard. Two argv shapes that still ingest are closed - chat index help and chat index /? - while memory search help stays a query, and the live repro against the real home is forbidden because running it causes the very harm the issue reports.
…, #140) cxc chat index --help ran a full ingest: --help was not a declared option, strict:false dropped it, and runChatIndex went straight to ingest. A command that looks read-only rewrote the user's index - during an audit it took the index from 28 files to 101. wantsHelp now answers at the top of main before any dispatch, and for chat index a bare positional help or /? does the same, while memory search help keeps searching for the word. strict:true makes a typo an error instead of a silent ingest. For #140, a dash-leading value for --cwd, --cwd-only, --home or --index-path is refused instead of swallowing the next flag and searching the wrong scope at exit 0, a missing --home directory exits non-zero, and the missing-root and missing-db conditions are now distinguishable and warn once each, deduped at the second searchStage1 call rather than by filtering the warnings array. The file header no longer claims the CLI never writes.
The ubuntu CI lane measures the suite and inventory.mjs --check --tests fails when the README badge disagrees. Every layer in this stack adds tests, so every layer publishes its own cumulative total; this one is 3097.
995a8ed to
45b8555
Compare
9128bd3 to
19de0ac
Compare
github-actions
Bot
changed the title
2 participants
cxc chat index --helpran a full ingest — during an audit it took the index from 28 files to 101. Help is now answered at the top ofmainbefore any dispatch,chat index helpand/?do the same whilememory search helpkeeps searching for the word, andstrict: truemakes a typo an error instead of a silent ingest. For #140 a dash-leading value is refused instead of swallowing the next flag and searching the wrong scope at exit 0, a missing--homeexits non-zero, and the missing-root and missing-db warnings are distinguishable and emitted once each.Closes #139
Closes #140
Stack (merge bottom-up)
codex/memory-recall-roadmapcodex/fix-recall-cwd-normalizationcodex/fix-memory-search-semanticscodex/fix-recall-cli-arg-hygienecodex/fix-chat-index-freshnesscodex/fix-recall-intent-regexcodex/fix-memory-write-gateYou are here: L3. Base is
codex/fix-memory-search-semantics. Review this PR's diff only — it is already scoped to this layer.Review focus
every argv shape that could still reach
ingest.Evidence
Every layer was planned to diff level before any code, audited by an independent
xai/grok-4.6reviewer, and implemented only after the audit's blockers were folded. Each new test was observed failing on the parent tip before it was shown passing; the per-layer receipt indevlog/_plan/260911_memory_recall_sweep/records the exact red output.Local gate:
npm run buildexit 0, andnpm testshowing exactly the two pre-existing environmental failures recorded in002_host_verification_baseline.md(hook-benchhardcodescwd: "/tmp", andcxc map --helpneedspy) and no third.Note on the target-branch check
Enforce PR target branchrequiresdev. Layers L1-L6 legitimately target the layer below, so that workflow will flag them. Do not retarget them todev— that would dissolve the stack. Merge bottom-up; each merge retargets the next child.